-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support join
URLs to generate OPcodes on phone, Fix URL scheme, "Paste" improvements
#1187
Support join
URLs to generate OPcodes on phone, Fix URL scheme, "Paste" improvements
#1187
Conversation
e-mission/e-mission-docs#1076 With the generation of OPcodes now occuring on the phone, several of the functions related to opcode generation will be moved from the join page to the phone. I put these in a separate file "opcode.ts" which will also pull in several of the functions from dynamicConfig that are related to validating and parsing opcodes While doing this, I adjusted the names of these functions to a unified convention, so let me describe where each one came from: generateRandomString: from join page's _getRandomString getStudyNameFromToken: from dynamicConfig's extractStudyName getSubgroupFromToken: from dynamicConfig's extractSubgroup getStudyNameFromUrl: from _getStudyName which was on both join page and dynamicConfig generate generateOpcodeFromUrl: from part of join page's validateAndGenerateToken (For reference, here is the current version of the join page that some of these functions are sourced from: https://github.com/e-mission/nrel-openpath-join-page/blob/main/frontend/index.html#L515) getTokenFromUrl and joinWithTokenOrUrl are new and help us handle 3 types of input: i) an actual token, ii) 'login_token' link that contains a token, and iii) 'join' link that gives parameters for a token to be generated on the phone
In the current version of the app, QR codes have to be scanned from within the app; scanning the QR code from the device's camera or any other external app will not successfully launch openpath. This used to work and was the method we used before we made a lot of changes to the onboarding flow. I found that the plugin relies on the existence of a global function (ie existing on the 'window' object). If we just supply this, the app can handle any emission:// link (or nrelopenpath:// link for the NREL version of the app) I'm also putting this function in the AppContext so it can be invoked in a "React"-friendly way downstream
Now that reading URLs or tokens, generating a token (if applicable), and joining a study is handled by handleOpenURL (and the functions in opcode.ts it calls), we can use this on the WelcomePage. Scanning or entering an opcode or URL on the WelcomePage should do the exact same thing as handling an emission:// link that was opened from some external location. getCode and loginWithToken are not needed anymore, scanCode just needs to call handleOpenURL. I saw an opportunity to make the "paste" flow easier by providing a shortcut; when "Paste code" is clicked the app will read the user's clipboard and try to join using that. Only if that fails will the paste modal need to be opened, as was the previous UX. I used cordova-clipboard for this.
I had this change locally and did not mean to commit it
-catch error that could be thrown from getTokenFromUrl -pass return value of false on either of the catch blocks -only show "proceeding with OPcode ..." if result from initByUser was true -ability to pass style through to AlertBar; wordBreak: break-all so that long OPcodes will not be forced onto one line
join
URLs to generate OPcodes on phone, Fix URL scheme, "Paste" improvements
Instead of hardcoding `emission://`, we can grab it from the package json. This way, the 'emission' string will only need to be changed in one place for other versions of the app. And since QrCode.tsx is able to handle either URLs or plain tokens, we can also simplify PopOpCode to just accept the plain token and pass this through to QrCode.tsx
Looking at these 2 functions I realized they can be combined and error handling unified, so I combined these to make a new function using async/await syntax. I chose to put the new function in dynamicConfig because this avoids a circular dependency at the module level (which is allowed but not tidy) Now, `dynamicConfig.ts` imports from `opcode.ts` but `opcode.ts` no longer imports anything from `dynamicConfig.ts`. Updated the tests for dynamicConfig to validate that the correct error messages are shown.
Because joinWithTokenOrUrl in dynamicConfig.ts calls functions from opcode.ts and there are tests for that, most of opcode.ts is already covered. However, I am adding a few more dedicated tests to cover more scenarios.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@JGreenlee can we add some stats to see how people actually provide us with the opcode - launch via URL or scan from app? That will help us smooth the onboarding flow further. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with merging this once we add the stats
We have multiple ways to initiate onboarding: - scan QR in the app - paste from clipboard in the app - entering to a textbox in the app - launching a nrelopenpath:// link from an external app (browser, camera) We want to keep track of how often these methods are used, as well as how often they are successful. First I added the 'onboard' stat which represents an attempt to onboard. The reading has 'configUpdated' (to indicate whether the attempt was successful), and 'joinMethod' ('scan', 'paste', 'textbox', or 'external') I also added "open_qr_scanner" and "paste_token" events (no readings) because the user might open the QR scanner but not scan any QR codes. An 'onboard' stat would not be recorded, but we still want to know if the user clicked "Scan QR" Lastly, I added 'onboarding_state' as a more general stat for evaluating our onboarding process. We should be able to compare the timestamp at each reading to see how long users take at each stage of the onboarding process – as well as how long the entire onboarding process takes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
add opcode.ts, extracting functions from dynamicConfig and the join page
e-mission/e-mission-docs#1076
With the generation of OPcodes now occuring on the phone, several of the functions related to opcode generation will be moved from the join page to the phone.
I put these in a separate file "opcode.ts" which will also pull in several of the functions from dynamicConfig that are related to validating and parsing opcodes
While doing this, I adjusted the names of these functions to a unified convention, so let me describe where each one came from:
generateRandomString: from join page's _getRandomString
getStudyNameFromToken: from dynamicConfig's extractStudyName
getSubgroupFromToken: from dynamicConfig's extractSubgroup
getStudyNameFromUrl: from _getStudyName which was on both join page and dynamicConfig
generate
generateOpcodeFromUrl: from part of join page's validateAndGenerateToken
(For reference, here is the current version of the join page that some of these functions are sourced from: https://github.com/e-mission/nrel-openpath-join-page/blob/main/frontend/index.html#L515)
getTokenFromUrl and joinWithTokenOrUrl are new and help us handle 3 types of input: i) an actual token, ii) 'login_token' link that contains a token, and iii) 'join' link that gives parameters for a token to be generated on the phone
implement global 'handleOpenURL' function
In the current version of the app, QR codes have to be scanned from within the app; scanning the QR code from the device's camera or any other external app will not successfully launch openpath. This used to work and was the method we used before we made a lot of changes to the onboarding flow.
I found that the plugin relies on the existence of a global function (ie existing on the 'window' object).
If we just supply this, the app can handle any emission:// link (or nrelopenpath:// link for the NREL version of the app)
I'm also putting this function in the AppContext so it can be invoked in a "React"-friendly way downstream
WelcomePage: handle tokens, 'join' / 'login_token' URLs, autopaste
Now that reading URLs or tokens, generating a token (if applicable), and joining a study is handled by handleOpenURL (and the functions in opcode.ts it calls), we can use this on the WelcomePage.
Scanning or entering an opcode or URL on the WelcomePage should do the exact same thing as handling an emission:// link that was opened from some external location.
getCode and loginWithToken are not needed anymore, scanCode just needs to call handleOpenURL.
I saw an opportunity to make the "paste" flow easier by providing a shortcut; when "Paste code" is clicked the app will read the user's clipboard and try to join using that. Only if that fails will the paste modal need to be opened, as was the previous UX.
I used cordova-clipboard for this.
revert commenting out '10.0.2.2' downloadURL
I had this change locally and did not mean to commit it
better error handling in opcode.ts
-catch error that could be thrown from getTokenFromUrl
-pass return value of false on either of the catch blocks
-only show "proceeding with OPcode ..." if result from initByUser was true
-ability to pass style through to AlertBar; wordBreak: break-all so that long OPcodes will not be forced onto one line
use URL_SCHEME from package.json; simplify PopOpCode.tsx
Instead of hardcoding
emission://
, we can grab it from the package json. This way, the 'emission' string will only need to be changed in one place for other versions of the app.And since QrCode.tsx is able to handle either URLs or plain tokens, we can also simplify PopOpCode to just accept the plain token and pass this through to QrCode.tsx
combine opcode.joinWithTokenOrUrl with dynamicConfig.initByUser
Looking at these 2 functions I realized they can be combined and error handling unified, so I combined these to make a new function using async/await syntax.
I chose to put the new function in dynamicConfig because this avoids a circular dependency at the module level (which is allowed but not tidy)
Now,
dynamicConfig.ts
imports fromopcode.ts
butopcode.ts
no longer imports anything fromdynamicConfig.ts
.Updated the tests for dynamicConfig to validate that the correct error messages are shown.
add tests for opcode.ts
Because joinWithTokenOrUrl in dynamicConfig.ts calls functions from opcode.ts and there are tests for that, most of opcode.ts is already covered. However, I am adding a few more dedicated tests to cover more scenarios.